Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add perWeek() interval #25

Closed

Conversation

marcoboers
Copy link
Contributor

This PR implements the perWeek() interval.

Related to #7

@Larsklopstra
Copy link
Member

The behavior Postgresql returns isn't expected, according to the docs:

week of month (1–5) (the first week starts on the first day of the month)

This will result in duplicate keys

@Larsklopstra
Copy link
Member

Will close this for now

@jtomek
Copy link

jtomek commented Mar 24, 2023

That's a pity.

@ziming
Copy link

ziming commented May 19, 2023

@Larsklopstra is it fine to merge this but throw an exception if the site is using postgres driver?

This will allow us MySQL users to use it.

After all you did throw an error if user is not on mysql, pgsql or sqlite

@mberneis
Copy link

+1 here - I need a week interval as well

@jtomek
Copy link

jtomek commented Jan 26, 2024

I don't have the resources to dig into this right not, but Chatgepete suggested this:

― ChatGPT 4:

The issue arises with PostgreSQL's interpretation of the week. PostgreSQL's date format YYYY-W returns the week of the month, which can lead to duplicate keys because multiple months may have the same 'week number' (e.g., the first week of January and the first week of February would both be week 1).

To resolve this and ensure each week is uniquely identified, you might consider using a format that combines the year and the week of the year instead of the week of the month. PostgreSQL supports the IW format for the ISO week number and the IYYY format for the ISO week-numbering year. Using this combination ensures that each week is uniquely identified across years.

You could change the PostgreSQL format string in PgsqlAdapter.php from 'YYYY-W' to 'IYYY-IW'. This format respects the ISO-8601 definition of a week: starting on Monday and belonging to the year that contains the majority of its days.

What do you think?

@danswiser
Copy link

danswiser commented Feb 16, 2024

@Larsklopstra

I can't speak for others but I would expect ->perWeek() to use the calendar week (1-52) instead of the month week (1-5) anyway.

Unless there's a majority that are actually looking for the week number of a month (1-5) then it sounds like we'd be fine implementing the previous comment suggestions.

@schmeits
Copy link

@Larsklopstra if you change the formatter of the week to "IYYY-IW" for PgSQL it's working fine...

See the docs:
IW | Week number of ISO 8601 week-numbering year (01-53; the first Thursday of the year is in week 1)
IYYY | ISO 8601 week-numbering year (4 or more digits)

@marcoboers marcoboers mentioned this pull request May 16, 2024
@marcoboers
Copy link
Contributor Author

I added the PostgreSQL fix in #63

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants